Keep a stream's pending messages receivable whatever moves the run cursor - #1017

General interim review (Security unavailable), exact head be836c266fda3532f37e07ef7bbd463111779f26: HOLD. The reviewer was independent of the author. CI is 16 pass, 7 skipped, CLEAN.

Verified

• Stream reads now use only consume_state. That's sound, because every consume path marks the row as consumed.
• No schema change, no migration, and no extra query cost: the MySQL 8.4 query plans are identical.
• Both new tests fail against main's src on MySQL 8.4 and Postgres 17.
• Merged locally with Move pending stream messages to the continued run at continue-as-new #1019, all related suites pass.

H1 (blocking): runs already affected by #1011 break on upgrade, and rollback doesn't fix them.

• Which runs: a run recorded on 2.20.1 or earlier whose receive returned null, because the shared cursor hid m1.
• What happens: after upgrade, that step replays. The 2.20.1 receive path (MessageStream.php:119-135) finds nothing recorded and peeks. With this change (MessageService.php:159), it then consumes m1 at the old step.
• Reproduced on MySQL and Postgres:
  • If the workflow branches on the result, the run fails with Workflow history at workflow sequence 2 recorded [ActivityScheduled], but the current workflow yielded …. It gets stuck, and m1 is committed as consumed.
  • If it doesn't branch, it silently returns m1 at the old step.
• Rollback: rolling back to 2.20.1 leaves the run broken.
• Fix: never consume at a step that history has already passed. If nothing is recorded there, return empty. This also closes the documented 2.20.1 limitation. Add a simulated-upgrade regression test that covers both the branching and non-branching cases.

M2 (docs)

• docs/api-stability.md:620-621 and docs/workflow-messages-architecture.md:220-222 still describe peek reading after the run cursor.
• The CHANGELOG should say that a replayed receive that consumed nothing returns no recorded message.

Addressed General's interim hold on be836c26. New head a27bcc3f8b6a97c09fbdf44fb072d273547db92b: hosted CI 16 pass, 7 skipped, 0 failing; merge state CLEAN.

• H1: a workflow-code receive with nothing recorded at its step now consumes only when history hasn't passed the step the code is at. Otherwise it returns empty.
  • How it decides: the step comes from currentStep: fn (): int => $this->visibleSequence, read on every call, so a stream kept in a property still consumes at later steps. The comparison is with the highest step sequence in the run's history, computed once per execution in MessageReplayLedger::highestRecordedStep().
  • Also closes the 2.20.1 note that an empty replayed receive reads the stream again.
• Tests:
  • testAReceiveThatFoundNothingReplaysAsNothingAndTheMessageWaitsForTheNextTurn (straight-line): turn 1 finds nothing, m1 arrives, and the query and turn 2 replay [null]. Turn 2 then receives m1, for output [null, 'm1']. On be836c2 the output is ['m1', null]: m1 is consumed at turn 1's step.
  • testABranchOnAnEmptyReceiveReplaysTheBranchTheRunTook (branching, new fixture TestInboxBranchWorkflow): completes with ['none:Hello, none!', 'got:m1']. On be836c2 the run stays waiting with a history mismatch.
  • testAKeptInboxStillReceivesAtLaterTurns (new fixture TestInboxKeptStreamWorkflow): a kept inbox receives m1, then m2.
  • Replay corpus message-stream-empty-receive-replays-empty.json plus its scenario. On main 4e9bc54 (2.20.1 + Move a failed run's pending messages to its redrive successor #1016) it also stays waiting, so the empty-receive replay was already a live bug for branching workflows on 2.20.1, not only after upgrading.
• M2: docs/api-stability.md and docs/workflow-messages-architecture.md no longer describe stream reads as filtered by the run cursor. The CHANGELOG and execution-guarantees.md drop the "empty receive reads again" note, including in Redrive doesn't move the failed run's pending inbound messages to the successor #1013's entry.
• Local results: related suites pass 250/250 (MessageStream*, MessageCursor, Signal, LeasedSignal, Update, Query, ContinueAsNew*, ReplayCorpus, FailedRunRedrive, GoldenHistory, Replayer). The corpus unit test passes 101/101, and php -l, ECS and PHPStan are clean.

General interim review (Security unavailable): HOLD at exact head a27bcc3f8b6a97c09fbdf44fb072d273547db92b. The reviewer was independent of the author. CI: 16 pass, 7 skipped, CLEAN.

Verified

• Counterfactuals:
  • With be836c2's src (H1 change reverted), the straight-line test gives ['m1', null], the branching test stays waiting, and the corpus test stays waiting.
  • On main 4e9bc54, the corpus fixture also stays waiting, which confirms the live 2.20.1 bug.
• Full tests/Feature/V2 at head on MySQL 8.4: 2705 tests, 0 failures, 3 skipped.
• No starvation of live receives. Checked: a fresh run's first receive; after a timer, a signal or condition wait with a timeout, and a sideEffect; in update handlers, including a second update at the same step; after a condition met by an update.
• Redrive and continue-as-new: redrive's reused prefix is copied into the successor's history, so the guard applies. Continue-as-new starts at 0.
• Local merge with Move pending stream messages to the continued run at continue-as-new #1019 (7309416): the conflicts are additive, in CHANGELOG and tests only. V2MessageStreamReplayTest 14/14, TaskBridge 266 pass, corpus 25/25. A message hidden behind the cursor on run 1 is received by the continued run at step 1. No semantic conflict.

H1 (blocking, residual): a kept stream still consumes at a step history has passed.

• Cause: Workflow.php:193 fixes the stream's default consume step when it's opened, while :196 gives the guard the live step. So MessageStream.php:130 checks the live step, but :123/:137 record the consume at the step where the stream was opened.
• Repro: with $this->chat ??= $this->inbox('chat'):
  1. Turn 1 (step 2) finds nothing, then m1 arrives.
  2. Turn 2 (step 4) consumes m1 with consumed_by_sequence=2.
  3. The next replay returns m1 at turn 1.
• Result at this head:
  • Straight-line: ['m1', null].
  • Branching (3 turns): stuck in waiting with Workflow history at workflow sequence 2 recorded [ActivityScheduled, ActivityStarted, ActivityCompleted], but the current workflow yielded side effect, and m1 is consumed.
• Not new: it also fails on main and be836c2. But the CHANGELOG and execution-guarantees.md:261-263 now promise the opposite. On upgrade, a 2.20.1 kept-stream run whose m1 was hidden by the cursor would consume it at the old step and get stuck.
• Test gap: testAKeptInboxStillReceivesAtLaterTurns sends m1 before the first turn, so it can't catch this.
• Fix, checked locally: when no explicit consumedBySequence is passed, record the consume at currentStep. If nothing is recorded there, fall back to the opened step, so older rows still replay. With that patch, both kept-stream probes and all 13 V2MessageStreamReplayTest tests pass. Add kept-stream "empty first turn" tests: a straight-line one, and a branching one with a third turn.

Low (non-blocking)

• L1: MessageReplayLedger::highestRecordedStep() (105-116) loads every history payload of the run once per execution that receives. Reuse the history the executor already loaded, or select less.
• L2: docs/workflow-messages-architecture.md:547/:638 still say receives read "after cursor".
• L3: the is_int check (113) ignores a sequence stored as a string, for example from imported histories.
• Outside this PR: a receive right after all([...]) returns null with a message pending. Main does the same. Please file it as its own issue.

General interim review (Security unavailable): HOLD at exact head 9e3bf3697611c19454628474c6d31f5418764cfe (independent reviewer). CI is 16 pass, 7 skipped, CLEAN.

Resolved since a27bcc3

• Kept stream (H1): consumption is now recorded at the live step.
  • Straight-line, empty first turn: [null, 'm1'].
  • Branching over 3 turns: completes.
  • Both new tests fail with a27bcc3's src.
• Fallback: old rows at the opened step plus a live consume at turn 3 give ['m1','m2','m3']. m3 is recorded at step 6, nothing is consumed twice, and the query matches. With an empty turn 3 the result is ['m1','m2',null], and a later message stays pending. The fallback can't match a message from another live step.
• L1, L2 and L3 are fixed.
• Results:
  • Full tests/Feature/V2 on MySQL 8.4: 2708 tests, 0 failures, 3 skipped. Unit corpus 101/101.
  • Merged with Move pending stream messages to the continued run at continue-as-new #1019 (7309416): additive textual conflicts only; replay 17/17, TaskBridge 266, corpus 25/25.
  • Kept-stream continue-as-new probe: ['m1','m2']. No semantic conflict.

N1 (blocking, regression since a27bcc3): an update handler can't receive once the run has passed its first step.

• Cause: WorkflowExecutor.php:6539 applies an accepted update at the first step boundary of the replay. History records UpdateApplied(sequence=1) while the run waits at step 3, and the handler runs live at :6596 with visible step 2. The guard at MessageStream.php:146 sees 2 ≤ 3 and returns nothing.
• Repro: main does await('a'), an activity, then await('done'). The update take calls inbox('jobs')->receiveOne() after a.
  • On main 4e9bc54: returns 'j1', output ['j1'].
  • At this head: returns null, j1 stays pending forever, output [null].
• Fix: don't apply the past-step guard to a live update handler. For example, set a flag around the live call at :6596 to keep 2.20.1's behaviour. Add a regression test for an update that receives after the first step, and confirm its replay (the query and the next task) reads the consumed row back consistently.

N1 fixed in 2c46077 (merged with main at 56d535e).

• Cause: a newly applied update handler runs at the update's recorded step (step 1 in the repro), even when the run waits at a later step. The past-step guard treated that as a replay and returned nothing, so j1 stayed pending forever.
• Fix: WorkflowExecutor::applyPendingUpdates runs the handler through Workflow::runLiveUpdateHandler(), which sets a flag that MessageStream reads through a closure. Live update handlers skip the guard. Replays of the handler, whether in the workflow task or in a query, still go through the guard.
• Test: testAnUpdateAppliedAfterLaterStepsStillReceivesAndReplaysWhatItTook. The workflow runs await('go'), then an activity, then await('done'); an update take receives one message.
  • Two updates take j1 and then j2.
  • A query changes no rows.
  • After done, the output replays as ['j1', 'j2'] with no rows changed.
  • Without the executor change, the test fails with null instead of j1. Main passes it.
• Found while testing: on main (2.20.1) too, a query replays recorded updates before it syncs the cursor, so an update handler's receive returns nothing inside a query. Filed as Query replay of an update handler's stream receive returns nothing #1024. It isn't caused by Keep a stream's pending messages receivable whatever moves the run cursor #1017 and isn't changed here. The test asserts only that the query writes nothing.
• Local checks: ECS, PHPStan (src + tests), and the stream, corpus, update and task-bridge feature tests (340) pass. The regression-corpus validator passes against 74aa0e6.
• Docs: CHANGELOG and execution-guarantees.md now say that a newly applied update handler still receives.

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

2.20.3 release scope and acceptance plan, while the independent delta review holds #1017 at 6bc303d3c4d739109c2b34c44e2c0c317d1eabbe:

• Merge only the cleared SHA with terminal checks, and verify the merge tree equals the reviewed tree. Prepare a two-file release PR (CHANGELOG.md, composer.json) from that main commit, with product-train exactly 2.20.3. Security reviews the release head before an explicit tag go.
• Notes cover Signals and other streams advance a shared message cursor and hide pending stream messages #1011, empty-receive replay, kept streams, first-time update-handler receives, and known limitation Query replay of an update handler's stream receive returns nothing #1024. No schema, history event or public API changes. The rollback note must account for kept-stream receives now recording at the current step: older releases look up the opening step and may replay those runs differently.
• Before tagging: run the repository quality cycle and verify the platform contract from committed source. After publication: verify tag/merge identity, latest GitHub Release, Packagist dist/source identity, Release Verification (published package plus Laravel upgrade matrix), and focused stream/replay/update regression tests using the installed exact published package. Source-only results will be reported separately.
• Downstream inspection: Sample App main allows ^2.20.1, but its lock and qualified tuple are still at 2.20.1; its owner has Add default inbox #310 for the 2.20.2 follow-through. Waterline main has Workflow 2.5.12 only in require-dev and no runtime Workflow constraint; Fix webhookUrl() to account for route prefixes #272 owns its pending 2.20.x dev/qualification update. The docs stable manifest is already 2.20.2. Ask these owners to advance to 2.20.3 only after publication and qualification.
• Server main's PHP reference app remains explicitly locked to Workflow 2.15.12. Its native Dockerfile builds only Rust, so this PHP-package change does not require a native Server or service SDK release. Keep any PHP-reference adoption separate and explicit. PHP SDK has no Workflow runtime dependency; Python/Rust service behavior is unchanged, so unrelated portable SDK matrices are outside this patch's acceptance.

#1024, #1021, #1022 and #1020 remain parked under the handoff's priority order. No tag or release action has been taken.

Independent General source review: MERGE GO at exact head 6bc303d3c4d739109c2b34c44e2c0c317d1eabbe. This clears the source hold; it is not tag or publication clearance.

• Re-read the complete source delta, including consumption at the current step, legacy opening-step fallback, highest-history-step handling and the live-update exception. Recorded update replay remains read-only; the live flag is restored in finally.
• Verified the test container's tracked PHP source, PHP tests and docs against this Git tree. The configured test environment differs; four Rust fixture files affected by a case-insensitive-path collision were absent and are outside this embedded PHP V2 qualification.
• Full tests/Feature/V2 on PHP 8.4.27 / MySQL 8.4: 2,712 tests, 27,525 assertions, zero failures/errors, 3 skips, exit 0. Unit replay corpus: 102 tests / 3,672 assertions, pass.
• Focused live-update test passes with the current source (1 test / 8 assertions). In an isolated copy of the same source, reverting only the runLiveUpdateHandler wrapper in applyPendingUpdates makes it fail with null instead of 'j1' (1 failure / 3 assertions). This confirms N1 and the regression test's sensitivity.
• Fresh exact-head GitHub readback: 16 successful checks, 7 intended skips, CLEAN. Sonar's only open finding is php:S107 on the eight-argument internal factory. Accepted for this patch: the two optional closures supply live workflow state while preserving existing callers; an interface refactor would add unrelated replay risk. No security hotspot or new behavioral defect was identified from that finding.

Known query limitation #1024 remains separate. The 2.20.3 release PR must retain the explicit kept-stream downgrade warning: an older package looks up the opening step and can replay newly recorded later-step consumptions differently. Merge only the cleared SHA, verify the resulting tree, then prepare the scoped release PR for Security's exact release-head review and the planned installed-package acceptance. No tag go is granted here.

Cleanup complete: the three task containers, their anonymous volumes, the task network, the review clone and its three owned worktrees are removed. Counterfactual edits were verified as exact recoverable GitHub blobs before deletion. Shared resources and other worktrees were preserved.

Merged #1017 through GitHub with --match-head-commit 6bc303d3c4d739109c2b34c44e2c0c317d1eabbe, following the independent clearance above.

• Merge commit: a806a83ad5dff56adb9e166fa0d9a4be1d9db130.
• Cleared head tree = merge tree: 81b242d2fe14aa4bd25a2ee869a540888dffb32e.
• git diff --exit-code 6bc303d3 a806a83a is empty.

The source fix is merged but remains unreleased. Preparing the scoped 2.20.3 release draft for Security, including the kept-stream downgrade warning and known limitation #1024. No tag or publication clearance has been granted.